Add tap controls; refactor AC_branches.jl by acostarelli · Pull Request #239 · Sienna-Platform/PowerOperationsModels.jl

acostarelli · 2026-08-15T03:20:24Z

Add support for tap control objectives, and large refactoring of AC_branches.jl

Copilot

Pull request overview

This PR refactors transformer tap handling by moving tap control behavior into the native branch construction path (gated by an enable_controls attribute and per-circuit TransformerControlObjective), while reorganizing/renaming related tests and tightening network-reduction “irreducible bus” logic for controlled devices and outages.

Changes:

Add tap-control variable/constraint plumbing to the native AC/DC branch models (including new voltage- and reactive-flow control constraints).
Restructure and expand transformer tap tests into separate “fixed tap” vs “controls” suites; remove legacy/disabled tap-control test files.
Refactor irreducible-bus selection to incorporate outage-monitored/outaged components and controllable transformers.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/test_voltage_control_tap_models.jl	Removes legacy VoltageControlTap model tests.
test/test_transformer_fixed_tap.jl	Adds fixed (off-nominal) tap physics tests across native models.
test/test_transformer_controls.jl	Adds transformer control-objective tests (tap variables + control bands).
test/test_power_flow_in_the_loop.jl	Removes commented PhaseShiftingTransformer PFitL test block.
test/test_postcontingency_mixed_outage_axes.jl	Updates outage pinning test to call the new helper.
test/test_native_transformer_tap.jl	Removes older fixed-tap + coefficient ground-truth tests (now relocated).
test/test_native_tapcontrol.jl	Removes disabled TapControl formulation tests.
test/test_native_network_reductions.jl	Removes commented PhaseAngleControl and tap regulated-bus error test blocks.
test/test_native_lpacc_model.jl	Re-enables/updates LPACC shunt validation gate test.
test/test_native_dcp_acp_models.jl	Re-enables `use_slacks` validation test for StaticBranchUnbounded.
test/test_device_branch_constructors.jl	Removes commented phase-shifting DC power flow test block.
test/runtests.jl	Removes DISABLED_TESTS entries (now empty).
test/Project.toml	Removes PowerFlows from test deps/sources (but tests still reference it).
test/includes.jl	Comments out `using PowerFlows` and `const PFS = PowerFlows` (but tests still reference `PFS`).
src/PowerOperationsModels.jl	Exports `ReactivePowerFlowControlConstraint`.
src/network_models/instantiate_network_model.jl	Refactors irreducible-bus selection; adds outage + controllable-transformer pinning.
src/core/network_formulations.jl	Removes old regulated-voltage/tap-current-form traits.
src/core/constraints.jl	Adds `ReactivePowerFlowControlConstraint` type.
src/ac_transmission_models/voltage_control_tap_models.jl	Removes legacy VoltageControlTap formulation implementation.
src/ac_transmission_models/branch_constructor.jl	Wires new tap-control variables/constraints into branch construction stages.
src/ac_transmission_models/AC_branches.jl	Implements tap-control variables, voltage/reactive control constraints, and tap-aware flow equations.

Suppressed comments (3)

test/includes.jl:39

PFS is still referenced by active tests (e.g. test/test_power_flow_in_the_loop.jl), but the alias is commented out. This will raise UndefVarError: PFS not defined at runtime.

const PSY = PowerSystems
const POM = PowerOperationsModels
const IOM = InfrastructureOptimizationModels
#const PFS = PowerFlows
const PSB = PowerSystemCaseBuilder
const PNM = PowerNetworkMatrices

test/Project.toml:23

PowerFlows is removed from [deps], but multiple active tests (not just commented blocks) still rely on it via the PFS alias (see test/test_power_flow_in_the_loop.jl). This will fail to instantiate the test environment.

This issue also appears on line 34 of the same file.

MathOptInterface = "b8f27783-ece8-5eb3-8dc8-9495eed66fee"
ParallelTestRunner = "d3525ed8-44d0-4b2c-a655-542cee43accc"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
PowerNetworkMatrices = "bed98974-b02a-5e2f-9fe0-a103f5c450dd"
PowerOperationsModels = "bed98974-b02a-5e2f-9ee0-a103f5c450dd"
PowerSystemCaseBuilder = "f00506e0-b84f-492a-93c2-c0a9afc4364e"
PowerSystems = "bcd98974-b02a-5e2f-9ee0-a103f5c450dd"

test/Project.toml:40

PowerFlows is removed from [sources], but it’s still required by the active PFitL tests. Without a source entry (when using custom git sources for the other Sienna packages), resolving the test environment may pull an incompatible version or fail outright.

[sources]
InfrastructureOptimizationModels = {rev = "main", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"}
InfrastructureSystems = {rev = "IS4", url = "https://github.com/Sienna-Platform/InfrastructureSystems.jl"}
PowerNetworkMatrices = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerNetworkMatrices.jl"}
PowerSystemCaseBuilder = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystemCaseBuilder.jl"}
PowerSystems = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystems.jl"}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

acostarelli · 2026-08-15T20:14:23Z

 function _validate_controlled_branch_not_reduced(
    network_model::NetworkModel,
-    devices::IS.FlattenIteratorWrapper{T},
-    formulation_name::String,
+    ::Type{T},
+    controlled_names,
 ) where {T <: PSY.ACTransmission}


transformer_models.jl isn't used. I will remove it when I implement phase shifting.

I believe a parallel merge is the only merge that can happen; series merges are blocked by specifying irreducible buses.

acostarelli · 2026-08-15T20:12:58Z

+            for circuit in PSY.get_circuits(transformer)
+                _control_enabled(circuit) || continue
+                _push_component_buses!(irreducible_buses, circuit)
+                push!(irreducible_buses, PSY.get_regulated_bus_number(circuit))
+            end


I think the convention we're going to go for now is the regulated bus number must always be a valid bus number, so 0 doesn't carry special importance.

That said, I agree maybe we shouldn't be pinning the regulated bus on all controls.

jd-lara · 2026-08-15T23:30:00Z

@acostarelli I stacked this on top of the PR with the network changes

acostarelli

Implementation review

acostarelli

Tests review

Have not checked that old tap tests that should be ported were ported.

acostarelli

Test port check. Seems fine.

…ible

…tage and q bounds

github-actions · 2026-08-19T03:11:57Z

Performance Results

Version	Precompile Time
Main	4.014770903
This Branch	3.978078645

Version	Build Time
Main-Build Time Precompile	58.968471135
Main-Build Time Postcompile	1.194906274
This Branch-Build Time Precompile	61.073104567
This Branch-Build Time Postcompile	1.200856903

Version	Solve Time
Main-Solve Time Precompile	200.360317122
Main-Solve Time Postcompile	172.389542674
This Branch-Solve Time Precompile	97.07061626
This Branch-Solve Time Postcompile	68.129853275

acostarelli · 2026-08-19T04:27:37Z

+    )
+_control_enabled(_) = false
+
+_tap_controlled(c::PSY.TransformerControlObjective) = c in (


Not sure how many of these helpers are needed anymore. I wrote them before the RepresentativeBranch refactor

acostarelli · 2026-08-19T04:30:37Z

@@ -184,72 +184,12 @@ end
 # -----------------------------------------------------------------------------


I have PFs disabled right now because there was some precompilation error. Haven't looked into it yet. It seemed like something upstream.

acostarelli · 2026-08-19T04:31:47Z

+# One entry per transformer arity. Every testset below runs the whole tuple, so the two
+# arities stay in lockstep; `circuit_indices` selects which circuit of the device carries
+# the control objective (the transformer for two-winding, the winding for three-winding).
+const TWO_WINDING_CASE = (


This is kinda verbose, but it works. I also haven't looked into the 3W-specific tests too closely but I think it mostly just reused the 2W tests.

acostarelli · 2026-08-19T05:01:05Z

+"""
+Used for specializing the device loop per concrete RepresentativeBranch.
+"""
+function _for_each_branch(f::F, reps) where {F}


Closures are unusual for Sienna, but we previously had the issue where the type of the branches we were iterating could've been Lines, MonitoredLines, TwoWindingTransformers, ThreeWindingTransformerCircuits, or some PNM.AbstractReductionAggregate. We need to guard the inner loop by a function so it can specialize.

That said, this is a pretty bare-bones method. Maybe that's okay. Maybe we combine to make _foreach_representative_branch and _foreach_branch ? The issue is a lot of call-sites need the branches vector before iterating so that they can build the names axis for a container, which is an annoying pattern I've encountered several times -- I wish we didn't have to initialize containers with names.

acostarelli requested review from jd-lara and m-bossart and a lite review from Copilot August 15, 2026 03:20

Copilot started reviewing on behalf of acostarelli August 15, 2026 03:20 View session

Copilot AI reviewed Aug 15, 2026

View reviewed changes

jd-lara changed the base branch from main to jd/network_matrix_consolidation August 15, 2026 23:29

jd-lara force-pushed the jd/network_matrix_consolidation branch from ce24265 to 7ebf176 Compare August 16, 2026 16:22

acostarelli commented Aug 17, 2026

View reviewed changes

Anthony Costarelli added 9 commits August 17, 2026 13:59

refactor voltage-only ac network flow code

027f186

refactor irreducible bus code; make controllable transformers irreduc…

ce8924d

…ible

rough tap control implementation; still need to run tests and add vol…

ad78ddf

…tage and q bounds

finished tap controls

be410db

claude reorganize tests and update tap tests

702114c

claude ported tests; tests now pass

1cd82e6

claude finished the impl

97785c1

remove unnecessary test

e11cd7b

formatting

7368dcd

acostarelli force-pushed the ac/transformer-control branch from dd4ad35 to 7368dcd Compare August 17, 2026 18:27

Anthony Costarelli added 2 commits August 17, 2026 14:46

readded bus reduction protection; again removed pfs for now

fc4726f

address review comments; refactor IVR, rename helpers

abc4c2c

acostarelli commented Aug 17, 2026

View reviewed changes

Comment thread test/test_transformer_controls.jl Outdated

Base automatically changed from jd/network_matrix_consolidation to main August 18, 2026 03:43

Anthony Costarelli added 5 commits August 18, 2026 10:24

rest of review comments

69ce8fe

claude refactor add_variables!

7ea2ee9

claude refactor to RepresentativeBranch

8084026

cleaned up claude's code; control constraints use new api

ad042e9

tests pass

7d45fb4

some bug fixes; claude 3w tests

ed02aec

Anthony Costarelli added 2 commits August 19, 2026 00:24

tests pass

4cfea67

formatting

1200d29

acostarelli commented Aug 19, 2026

View reviewed changes

acostarelli changed the title ~~Add tap controls~~ Add tap controls; refactor AC_branches.jl Aug 19, 2026

acostarelli commented Aug 19, 2026

View reviewed changes

		@@ -184,72 +184,12 @@ end
		# -----------------------------------------------------------------------------

Uh oh!

Conversation

acostarelli commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

acostarelli Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

jd-lara commented Aug 15, 2026

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acostarelli Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

acostarelli commented Aug 15, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading